⚠️ Known Issues — intentionally skipped tests

Source: docs/known_issues.md

Known Issues

Tests intentionally excluded from CI. Source of truth for the Known
Issues section the L1 workflow appends to each ef-tests job summary
and posts as a sticky PR comment.

EF Tests — Stateless coverage narrowed to EIP-8025 optional-proofs

make -C tooling/ef_tests/blockchain test calls test-stateless-zkevm
instead of test-stateless. The zkevm@v0.3.3 fixtures are filled against
bal@v5.6.1, out of sync with current bal spec; the broad target trips ~549
fixtures. Re-broaden once the zkevm bundle is regenerated.

🤖 Kimi Code Review

The PR implements noopTracer support correctly, following existing patterns in the codebase. The implementation properly executes transactions without recording trace data, matching geth's behavior.

Minor observations:

1. crates/blockchain/tracing.rs:113-114 - The Arc<Mutex<VM>> wrapping is necessary if timeout_trace_operation spawns blocking tasks (requiring Send), though it adds slight overhead for sequential execution. This is the correct defensive approach for async timeout handling.

2. crates/blockchain/tracing.rs:108 - Consider verifying that rerun_block(&block, Some(0)) correctly initializes the block state without executing any transactions (i.e., applies only the block reward/coinbase state changes if applicable).

3. crates/vm/backends/levm/tracing.rs:108 - Ensure LevmCallTracer::disabled() (not shown in diff) properly disables all tracing hooks to avoid performance overhead during execution.

4. Error handling consistency - The mutex poisoning error conversion to EvmError::Custom at line 116 is acceptable, though consider using a more specific error variant if one exists for synchronization failures.

Security/Performance:

• The reexec parameter bounds prevent unbounded state reconstruction (DoS vector), and the timeout parameter prevents long-running traces. Both are handled correctly.
• No state corruption risks identified; the mutable database reference is properly scoped within each transaction execution.

Verdict: LGTM. The implementation correctly follows the established patterns for tracer integration and properly handles the async timeout requirements.

Automated review by Kimi (Moonshot AI) · kimi-k2.5 · custom prompt

Lines of code report

Total lines added: 100
Total lines removed: 0
Total lines changed: 100

+-------------------------------------------+-------+------+
| File                                      | Lines | Diff |
+-------------------------------------------+-------+------+
| ethrex/crates/blockchain/tracing.rs       | 289   | +48  |
+-------------------------------------------+-------+------+
| ethrex/crates/networking/rpc/tracing.rs   | 336   | +21  |
+-------------------------------------------+-------+------+
| ethrex/crates/vm/backends/levm/tracing.rs | 344   | +15  |
+-------------------------------------------+-------+------+
| ethrex/crates/vm/tracing.rs               | 106   | +16  |
+-------------------------------------------+-------+------+

🤖 Claude Code Review

Now I have all the context needed for a thorough review. Let me compile the findings.

Review: feat(l1): add noopTracer for debug_traceTransaction and debug_traceBlockByNumber

Overall Assessment

The implementation is correct and closely mirrors the patterns established by callTracer and prestateTracer. The core logic is sound, the serde mapping is accurate, and no security or consensus issues are introduced. A few minor points below.

Correctness

State accumulation in trace_block_noop — correct.
vm.rerun_block(&block, Some(0)) runs prepare_block (system calls, etc.) and stops before tx 0, matching the pattern in trace_block_calls (line 67) and trace_block_prestate (line 195). Because LEVM::trace_tx_noop borrows db mutably and vm.execute() commits state into db.current_accounts_state, each subsequent tx in the loop correctly sees the accumulated state from all previous txs. This is consistent with how trace_tx_prestate is handled.

Serde mapping — correct.
#[serde(rename_all = "camelCase")] on TracerType maps NoopTracer → "noopTracer", which matches Geth's tracer name. ✓

Empty return value — correct.
serde_json::json!({}) per-tx matches Geth's noopTracer output format. ✓

Error propagation — correct.
vm.execute()? propagates real VM errors; transaction reverts surface as Ok(ExecutionResult::Reverted{...}) inside the VM, not as Err, so the noop path handles them the same way as the other tracers.

Issues

1. Missing explanatory comment on rerun_block call (crates/blockchain/tracing.rs, new trace_block_noop)

The analogous methods have comments that explain the Some(0) argument:

• trace_block_calls (line 67): "// Run anything necessary before executing the block's transactions (system calls, etc)"
• trace_block_prestate (line 195): "// Run system calls but stop before tx 0"

trace_block_noop has none, making it easy to misread as a no-op call or as incorrectly starting at tx 0. Adding the same comment would make the intent immediately clear.

2. Asymmetric return type in trace_block_noop (crates/blockchain/tracing.rs, line 117)

The two existing block tracers return Vec<(H256, T)>:

• trace_block_calls → Result<Vec<(H256, CallTrace)>, ChainError>
• trace_block_prestate → Result<Vec<(H256, PrestateResult)>, ChainError>

trace_block_noop returns Result<Vec<H256>, ChainError> — dropping the tuple — which forces the RPC handler to reconstruct BlockTraceComponent manually rather than using the existing From<(H256, TxTrace)> impl. Returning Result<Vec<(H256, ())>, ChainError> or even Result<BlockTrace<serde_json::Value>, ChainError> would keep the API surface uniform and allow the RPC handler to use the same .map(Into::into).collect() pipeline as the other arms. As written it works, but it creates a one-off pattern that's easy to diverge from in future tracers.

3. block.as_ref() — redundant but harmless (crates/blockchain/tracing.rs, line 133)

let tx_hash = block.as_ref().body.transactions[index].hash();

Arc<Block> already Derefs to Block, so .as_ref() is unnecessary. This is copied verbatim from trace_block_calls (line 77) and trace_block_prestate (line 204), so it's at least consistent — but both the old and new sites could drop .as_ref().

Style / Minor

• The error strings "Transaction not Found" and "Block not Found" (capital F) appear in trace_transaction_noop lines 101 and 105. These are directly copied from trace_transaction_calls (lines 31, 35) and trace_transaction_prestate (lines 160, 164), so they are consistent within the codebase — no new inconsistency introduced.

• trace_tx_noop at the LEVM level (crates/vm/backends/levm/tracing.rs:97) correctly reuses LevmCallTracer::disabled(), the same disabled tracer used by trace_tx_prestate. No duplication of tracer logic. ✓

Summary

No bugs, no security issues, no consensus risk. Two actionable suggestions: add the rerun_block explanatory comment to trace_block_noop, and consider aligning the return type of trace_block_noop with Vec<(H256, T)> for API consistency. Everything else is correct and idiomatic for this codebase.

Automated review by Claude (Anthropic) · sonnet · custom prompt

Greptile Summary

This PR adds noopTracer as a third TracerType option for debug_traceTransaction and debug_traceBlockByNumber, completing the standard Geth tracer triad alongside callTracer and prestateTracer. The implementation is wired consistently through all four layers (LEVM, Evm façade, Blockchain, RPC handler) following the same Arc/Mutex/timeout pattern established by the existing tracers.

• LEVM::trace_tx_noop reuses LevmCallTracer::disabled() (same as trace_tx_prestate) and simply calls vm.execute() without capturing any output, correctly exercising the trace harness without recording per-step data.
• TracerType::NoopTracer is deserialized from "noopTracer" (correct via rename_all = "camelCase") and returns {} for single-transaction requests or [{txHash, result:{}}] for block requests, matching Geth's specified output format.
• State accumulation across transactions in trace_block_noop is correct — each trace_tx_noop call mutates &mut self.db in-place, so subsequent transactions see the prior transactions' state changes.

Confidence Score: 5/5

Safe to merge — the noopTracer is a thin, read-only addition that runs transactions through the existing EVM harness without any state commits to storage or new data structures.

The four changed files each follow the established pattern for existing tracers character for character. State management, timeout wrapping, Arc/Mutex usage, serde deserialization, and JSON output format are all consistent with the callTracer and prestateTracer implementations that are already in production. No edge cases in the new code paths deviate from the existing tracers.

No files require special attention.

_{Reviews (1): Last reviewed commit: "feat(l1): add noopTracer for debug_trace..." | Re-trigger Greptile}

🤖 Codex Code Review

No blocking findings.

Residual risk:

1. The new noopTracer branches at crates/networking/rpc/tracing.rs and crates/networking/rpc/tracing.rs add public RPC behavior without regression coverage. Because block mode depends on sequential VM state mutation in crates/blockchain/tracing.rs, I’d add tests for both endpoints, ideally with a block where tx 2 depends on tx 1’s writes, to lock in both the {} response shape and the per-tx state progression.

The implementation itself is aligned with the existing call/prestate tracing flow; I didn’t spot an obvious EVM/gas-accounting, consensus, or security regression in the noop path from source inspection.

I couldn’t run cargo check in this sandbox because ~/.rustup / ~/.cargo are read-only here and the workspace needs git-backed dependencies.

Automated review by OpenAI Codex · gpt-5.4 · custom prompt

Follow-up commit 705073046 folds in two of the points raised:

• Added the // Run system calls but stop before tx 0 comment in trace_block_noop to match the other block tracers.
• Changed trace_block_noop to return Vec<(H256, ())> for symmetry with trace_block_calls / trace_block_prestate; RPC handler now uses the existing BlockTraceComponent From impl.

Skipped intentionally:

• block.as_ref() redundancy (Claude review): kept verbatim from the call/prestate tracers since Arc<Block> already Derefs. Cleaning it up cleanly would mean editing all three block tracers, which is out of scope for this PR — happy to do it as a separate cleanup.
• RPC-level regression tests (Codex review): no precedent in this crate — neither callTracer nor prestateTracer have RPC-handler tests, only LEVM-backend tests of their substantive behavior. There isn't substantive behavior to test at the LEVM level for noopTracer (it's vm.execute() with LevmCallTracer::disabled(), which is already exercised by trace_tx_prestate). Adding RPC-level harness scaffolding here would set a new precedent for one tracer; better as a separate "add tracer test harness" PR if the team wants it.

Also: kept #[allow(clippy::enum_variant_names)] instead of renaming variants to Call/Prestate/Noop — that route would require three per-variant #[serde(rename = "...")] attributes to keep the wire format ("callTracer", etc.) intact, which trades one allow for three renames.